Feat / Extend minimum browser support #519
Conversation
|
@langemike is "Chrome 68" something you need for Smart TV devices? |
Correct! |
platforms/web/src/index.tsx
Outdated
| @@ -1,3 +1,5 @@ | |||
| import 'core-js/es/array/flat-map'; | |||
| import 'core-js/es/object/from-entries'; | |||
There was a problem hiding this comment.
@langemike do you think we can import it conditionally based on the environment variable (APP_LEGACY_BROWSER)?
There was a problem hiding this comment.
I would not limit this env-var only to the conditional imports, which makes the approach more close like I originally intended, but I think we should keep it simple for now (at least) and make it more flexible if use-cases of demand for this increases.
There was a problem hiding this comment.
My main concern here is the "polyfills" file we additionally get when we add Chrome legacy support. Even if we don't need it.
There was a problem hiding this comment.
I revised the approach. Legacy support is now opt-in by providing APP_LEGACY_BUILD=1 during yarn build.
A polyfills.js is still created because some dependencies have imported core-js internally.
Without legacy support (smaller files):
build/public/manifest.json 0.49 kB │ gzip: 0.29 kB
build/public/index.html 2.97 kB │ gzip: 0.81 kB
build/public/assets/default_avatar-D3fd0lZl.png 26.96 kB
build/public/assets/style-xyq8hHtg.css 164.70 kB │ gzip: 29.50 kB
build/public/assets/polyfills-BTIFuIxg.js 108.89 kB │ gzip: 43.26 kB │ map: 676.28 kB
build/public/assets/react-Dl1qj1q7.js 143.24 kB │ gzip: 46.11 kB │ map: 351.28 kB
build/public/assets/inplayer-Caq7sMti.js 313.25 kB │ gzip: 92.39 kB │ map: 708.10 kB
build/public/assets/index-BJFB1YLe.js 342.80 kB │ gzip: 103.81 kB │ map: 1,260.17 kB
build/public/assets/vendor-oOs1bjTA.js 1,615.64 kB │ gzip: 431.98 kB │ map: 4,926.55 kB
With APP_LEGACY_BUILD=1 (bigger files):
build/public/manifest.json 0.49 kB │ gzip: 0.29 kB
build/public/index.html 2.97 kB │ gzip: 0.81 kB
build/public/assets/default_avatar-D3fd0lZl.png 26.96 kB
build/public/assets/style-xyq8hHtg.css 164.70 kB │ gzip: 29.50 kB
build/public/assets/polyfills-DtCB2TJL.js 90.71 kB │ gzip: 37.81 kB │ map: 567.16 kB
build/public/assets/react-Ba7Lyyxn.js 143.28 kB │ gzip: 46.12 kB │ map: 351.34 kB
build/public/assets/inplayer-CHHsr38C.js 313.25 kB │ gzip: 92.39 kB │ map: 708.10 kB
build/public/assets/index-CuUwpo7s.js 343.22 kB │ gzip: 104.00 kB │ map: 1,260.34 kB
build/public/assets/vendor-DQoT3ipD.js 1,615.76 kB │ gzip: 432.01 kB │ map: 4,926.75 kB
There was a problem hiding this comment.
For some reason they are smaller with APP_LEGACY_BUILD, but the approach itself looks good 👍 Thank you.
There was a problem hiding this comment.
I think the file listings are switched in the comment above. The added polyfills make the polyfills file larger when APP_LEGACY_BUILD is enabled.
langemike
force-pushed
the
feat/vite-minimum-browser-support
branch
2 times, most recently
from
dc1022c
to
bc6b79a
Compare
|
We discussed the knip ignore and virtual + relative path import via Slack and are planning to make the following (hopefully last) changes:
resolveId(id) {
if (id.includes('virtual:polyfills')) {
return '\0' + id;
}
},
load(id) {
if (id.includes('\0virtual:polyfills')) {
return enabled ? `import './polyfills';` : 'export default {};';
}
}, |
langemike
force-pushed
the
feat/vite-minimum-browser-support
branch
from
bc6b79a
to
0e7bb82
Compare
|
@ChristiaanScheermeijer I ammeded my previous commit and performed the approach you suggested. I removed the ignore knip-config and put |
We want to support a broader user base by extending our browser support to
chrome68and up.Vite internally uses
['es2020', 'edge88', 'firefox78', 'chrome87', 'safari14']. So the only difference is the supported chrome browsers. See https://vitejs.dev/guide/build#browser-compatibility for reference.I only needed to write a CSS fallback for the
dvhunit because it was the only unsupported CSS feature we have ran into.This change also contains a variable injection fix for
index.htmlwhich has been introduced (with the upgrade to vite 5, I assume).We also did some internal experiments with @vitejs/plugin-legacy, but this change has a bigger impact on our bundle/build. If we even want to broaden our browser support further than
chrome68then@vitejs/plugin-legacywould definitely be a viable option.I've tested this on Chrome 68 using browserstack.
Our original PR: Videodock#186
Our
@vitejs/plugin-legacyexperiment PR (for reference): Videodock#182